Skip to content

test: add tests for miscellaneous ioctl functions#1034

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
MaisenbacherD:test-misc
Jul 8, 2025
Merged

test: add tests for miscellaneous ioctl functions#1034
igaw merged 1 commit intolinux-nvme:masterfrom
MaisenbacherD:test-misc

Conversation

@MaisenbacherD
Copy link
Copy Markdown
Contributor

Use the mock ioctl infrastructure to test the functions in ioctl.h that issue miscellaneous ioctl commands.

@MaisenbacherD
Copy link
Copy Markdown
Contributor Author

Going to fix the failing test case soon :)

@MaisenbacherD MaisenbacherD force-pushed the test-misc branch 2 times, most recently from b84cacf to 860893a Compare July 8, 2025 08:15
Use the mock ioctl infrastructure to test the functions in ioctl.h that
issue miscellaneous ioctl commands.

Signed-off-by: Dennis Maisenbacher <[email protected]>
Copy link
Copy Markdown
Contributor Author

@MaisenbacherD MaisenbacherD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy to get some feedback, specifically on the highlighted section :)

Comment thread test/ioctl/misc.c
Comment on lines +418 to +419
_cleanup_free_ struct nvme_lba_status *lbas = NULL;
_cleanup_free_ struct nvme_lba_status *expected_lbas = NULL;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igaw Is this use of _cleanup_free_ idiomatic? I am happy to hear your thoughts on this. :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It avoids the common pitfall that the error path is never really tested and usually something gets leaked. So yes, if the scope of the allocation is within a block, it makes sense to use these cleanup function. FWIW, even the kernel started to use these :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add something like a util.h for the tests later.

@igaw igaw merged commit 4cef519 into linux-nvme:master Jul 8, 2025
12 checks passed
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Jul 8, 2025

Thanks!

@MaisenbacherD MaisenbacherD deleted the test-misc branch July 8, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants